Add Btrfs fallback for other Linux distros by locnnil · Pull Request #956 · canonical/workshop

locnnil · 2026-07-21T22:29:50Z

Description

Enables Btrfs fallback for other Linux distributions.

ZFS is the preferred storage driver, but it isn't available everywhere: WSL, SUSE, Arch, Fedora, and other distributions may ship without it. Keying the fallback off the distribution name is too coarse; it assumes every non-Ubuntu host lacks ZFS and every Ubuntu-derived one has it, neither of which holds for Debian and its derivatives.

Instead, detect whether the host provides a usable ZFS kernel module, already loaded (present at /dev/zfs AND /sys/module/zfs ) or loadable for the running kernel per modules.dep(5) (/lib/modules/$(uname -r)/modules.dep) or modules.builtin (/lib/modules/$(uname -r)/modules.builtin) and keep ZFS only then, falling back to btrfs otherwise. Only the kernel module is checked: LXD ships as a snap that bundles the zpool/zfs userspace tools, so a loaded or loadable module is what actually determines usability from the host side, tested on Arch Linux and openSUSE tumbleweed.

Closes: workshop does not work on non-Ubuntu systems #742

Self-review quick check

Make decisions that cost a lot to reverse explicit in the PR description.
Avoid nested conditions.
Delete dead code and redundant comments. TODO: check if it is okay to delete isWSL()
Normalise symmetries by sticking to doing identical things identically.

// one way to handle errors
if err := f(); err != nil {
   ...
}

// one way to handle multiple returns
val, err := f()
if err != nil {
   ...
}
...

Check that coupled code elements, files, and directories are adjacent. For example, test data is stored as close as possible to a test.
Put variable declaration and initialisation together.
Divide large expressions into digestable and self-explanatory ones. Use multiple variables if required.
Put a blank line between two logically different chunks of code.
Follow the style guide for new error messages.

Docs

Procedure:

I have checked and added or updated relevant documentation.
I have checked and added or updated relevant release notes.
I have included the technical author in the review.

Content:

Headings and titles accurately describe the content.
New and updated pages include correct metadata.
Documentation tests are added or updated where applicable (for tutorial/ and how-to/ sections).
Documentation follows the style guide.
If needed, docs/.coverage.yaml updated, coverage tags added (.. artefact).

Or:

I confirm the PR has no implications for documentation.

jonathan-conder · 2026-07-21T22:35:16Z

It looks to me like the way LXD does it is just trying to load the zfs module; maybe we should follow their lead on that

jonathan-conder · 2026-07-21T22:37:40Z

I think we also need to be more careful about where we use the storagePoolDriver variable. Currently we can assume it's accurate, since whether the host is WSL or not is unlikely to change. The availability of the ZFS module can easily change, though; we need a separate mechanism to indicate "what's the current driver of the workshop pool."

locnnil · 2026-07-22T00:23:10Z

I think we also need to be more careful about where we use the storagePoolDriver variable. Currently we can assume it's accurate, since whether the host is WSL or not is unlikely to change. The availability of the ZFS module can easily change, though; we need a separate mechanism to indicate "what's the current driver of the workshop pool."

So, considering the case where ZFS was available when the pool was created but later stops being present, most likely because the user installed a different kernel that doesn't ship the ZFS module. What should we do with the now-unusable ZFS pool?

I can think of those options:

Put the daemon into a degraded state and add an actionable error message like "restore the ZFS-capable kernel, or remove the pool to recreate it as Btrfs"
Warn the user with an actionable message like "Old ZFS pool detected on system without support for ZFS on path <path-to-zfs-pool>" but create a new Btrfs pool under a different name.

I believe that the first option is better, right?

jonathan-conder · 2026-07-22T01:37:09Z

I believe that the first option is better, right?

I agree, but we should not suggest removing the pool, it will probably break the workshops using it (if it's even possible to remove it). Instead maybe it's best to reinstall the Workshop snap.

We just need to test what actually happens in that scenario, maybe LXD will fail to start and we probably handle that already.

locnnil · 2026-08-14T01:50:14Z

@jonathan-conder I've implemented the requested change in 54bf6dd, which stops assuming a fixed state for ZFS availability. I then tested the degraded-state behavior for the scenario where ZFS is present at first but later vanishes.

I ran this on an Ubuntu Resolute LXD VM, reproducing a workshop pool created on ZFS whose kernel module later becomes unavailable after a kernel change.

Reproduction

Started with ZFS available, so the workshop pool was created on the zfs driver.
Built and installed a .deb for a newer kernel (7.2.0-rc7) that doesn't ship the ZFS module.
Removed the previous kernel and its ZFS module package (linux-main-modules-zfs-7.0.0-29-generic).
Rebooted into the new kernel and ran workshop list.

Result

Instead of surfacing a raw LXD error, the daemon goes into degraded mode with an actionable message:

$ workshop list
error: system is not healthy: cannot use the "workshop" storage pool, its storage driver may be unavailable (for example a ZFS pool after booting a kernel without the ZFS module): Error loading "zfs" module: Failed running: modprobe -b zfs
: exit status 1 (modprobe: FATAL: Module zfs not found in directory /lib/modules/7.2.0-rc7)
Boot into a kernel that provides the pool's storage driver, or reinstall Workshop to recreate the pool on an available driver

Recovery

The ZFS pool can't be deleted through LXD while the module is missing (lxc storage delete loads the driver, which fails), so the recovery is to reinstall Workshop: this recreates the workshop pool on an available driver (Btrfs) for a host that no longer provides ZFS.

On adding a Spread test

Let me know if you think we should add a Spread test covering this case. Two caveats:

For my manual test I had to build and install an upstream kernel, since simply uninstalling linux-main-modules-zfs-7.0.0-xx-generic isn't enough to reproduce a realistic scenario. For an automated test we could instead force the broken state, making it reproducible by just forcefully removing a package.
The bigger drawback is that this would require Spread to run on LXD VMs instead of containers. That would be a significant change to the current testing infrastructure.

jonathan-conder · 2026-08-16T22:17:22Z

 		inst.Devices = map[string]map[string]string{}
 	}
-	if err := mergeDevices(inst.Devices, snapshot.Sdks, name); err != nil {
+	usesZFS, err := poolUsesZFS(conn)


this adds an additional API call to these operations, do we have any info on the impact it has on latency? we could alternatively cache the storage pool driver at launch

With this change cbe0b61 poolUsesZFS reads from a cached value.

jonathan-conder · 2026-08-16T23:15:41Z

The ZFS pool can't be deleted through LXD while the module is missing (lxc storage delete loads the driver, which fails), so the recovery is to reinstall Workshop: this recreates the workshop pool on an available driver (Btrfs) for a host that no longer provides ZFS.

I don't see how reinstalling Workshop will help. If LXD is unable to delete the pool then it will still be there after the reinstall.

For my manual test I had to build and install an upstream kernel, since simply uninstalling linux-main-modules-zfs-7.0.0-xx-generic isn't enough to reproduce a realistic scenario. For an automated test we could instead force the broken state, making it reproducible by just forcefully removing a package.

It's not reasonable to do that in a test. It's a rare enough case that I think some manual recovery steps are OK.

The bigger drawback is that this would require Spread to run on LXD VMs instead of containers. That would be a significant change to the current testing infrastructure.

we already do this, it's not a change at all

locnnil · 2026-08-29T01:13:18Z

I don't see how reinstalling Workshop will help. If LXD is unable to delete the pool then it will still be there after the reinstall.

Yeah, I think in this case you would need to reinstall LXD and Workshop, but this seems too drastic. Anyway, I've tested, and when the ZFS-capable kernel is restored, the workshop will be working again.

jonathan-conder · 2026-09-04T00:10:27Z

+// re-querying LXD (which reloads the driver's module). The pool's driver is
+// fixed at creation, so the cached value stays accurate for the daemon's run.
+var poolDriverCache struct {
+	sync.RWMutex


do we need a mutex here? I think anything that looks at the storage driver should run after the value has been initialised.

jonathan-conder

Ok I think we're nearly there, just a couple of minor comments below, and I still think we should check for storagePoolDriver != existingPool.Driver in ensureBackendReady()

Copilot

Copilot review overview

🟡 Changes recommended

Critical storage-driver race and pool-validation issues remain, and fallback integration coverage is missing.

Review effort: Lite
Findings: 1 · 2

Open (3)

Prevent driver races and changes during pool recovery · New
Update documentation and diagrams for Btrfs support · New
Capitalize filesystem name as Btrfs · New

What changed in this PR

Adds dynamic ZFS-preferred storage selection with Btrfs fallback based on LXD capabilities, plus tests and documentation updates.

Changes:

Selects and tracks the storage pool driver.
Adds driver-selection test coverage and test exports.
Updates tutorial, reference, and architecture documentation.

File	Summary
`internal/workshop/lxd/lxd_backend.go`	Implements storage-driver selection and pool initialization.
`internal/workshop/lxd/lxd_backend_test.go`	Tests preferred-driver selection and fallback behavior.
`internal/workshop/lxd/export_test.go`	Exposes driver-selection logic for tests.
`docs/tutorial/part-1-get-started.rst`	Documents storage fallback prerequisites.
`docs/reference/workshops.rst`	Updates storage-driver documentation.
`docs/explanation/architecture/components.rst`	Updates the storage architecture description.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot

🟡 Changes recommended

Approval is blocked by an unresolved critical data race during LXD recovery, along with documentation inconsistencies about Btrfs capitalization and ZFS-only behavior.

1 open issue

Synchronize storage driver recovery with backend goroutines · New

1 previously missed issue

In code that hasn't changed since last review

Capitalize Btrfs consistently in user-facing documentation

docs/explanation/architecture/components.rst:262

This prose uses lowercase btrfs, whereas the tutorial and the new code comment use Btrfs. Keep the filesystem name consistently capitalized in user-facing documentation; reserve lowercase for the LXD driver identifier.

3 resolved since last review

_{Review effort: Lite}

Copilot

🔵 Needs a closer look

Approval is blocked by an unresolved race that can expose a newly created ZFS pool before its driver is published, along with unsupported pre-existing pool acceptance and inconsistent backend-specific documentation.

1 open issue

Synchronize storage driver recovery with backend goroutines

2 previously missed issues

In code that hasn't changed since last review

Publish the pool driver before concurrent backend operations

internal/workshop/lxd/lxd_backend.go:374

The atomic load/store removes the data race, but it does not close the publication window: after CreateStoragePool completes and before setPoolDriver runs, a concurrently executing task can enter mergeDevices or copyInstance and observe poolDriver() == "". On a newly created ZFS pool that omits initial.zfs.promote, so the first snapshot/copy can use the Btrfs path and fail or lose the required ZFS promotion. Publish the driver before the pool becomes usable, or serialize pool creation/recovery with backend operations.

Reject existing pools with unsupported drivers

internal/workshop/lxd/lxd_backend.go:408

When a workshop pool already exists and poolDriver() is still empty, this branch adopts whatever LXD driver it finds. That allows a pre-existing dir, lvm, or other pool to be silently accepted even though preferredDriver and the documented behavior only support ZFS and Btrfs; the snapshot behavior then no longer matches the backend's assumptions. Validate existingPool.Driver against the supported drivers before recording it (or return an actionable error).

_{Review effort: Lite}

Copilot

🟡 Changes recommended

Active critical issues remain, including an unused import that prevents compilation and insufficient validation of existing or unknown storage drivers, which can cause incorrect snapshot behavior.

5 open issues

_{Review effort: Lite}

Copilot

🟡 Changes recommended

The change leaves two active documentation inconsistencies: an unconditional ZFS/LZ4 capability claim on the Btrfs path and a contradictory developer prerequisite that still requires ZFS.

3 open issues

1 resolved since last review

Reject existing pools with unsupported drivers

_{Review effort: Lite}

+with automatic btrfs fallback otherwise,
+for example on Windows Subsystem for Linux (WSL).


 |ws_markup| is supported on Ubuntu
 and other :program:`snap`-enabled Linux distributions;
-it is also compatible with Windows Subsystem for Linux (WSL2),
-where it uses Btrfs instead of ZFS for storage.
+it is also compatible with Windows Subsystem for Linux (WSL2).
+For storage, it uses ZFS where |ws_markup| detects that ZFS is available,
+and otherwise falls back to btrfs.


Copilot

🟡 Changes recommended

Active documentation inconsistency and stale storage-driver caching remain unresolved, including a moderate correctness risk when recreating a deleted pool.

3 open issues

1 resolved since last review

Synchronize storage driver recovery with backend goroutines

_{Review effort: Lite}

Copilot

🔵 Needs a closer look

The documentation remains internally contradictory because several sections still present ZFS as universal despite newly documented Btrfs support, leaving an active correctness issue to resolve.

3 open issues

_{Review effort: Lite}

Copilot

🟡 Changes recommended

Active documentation inconsistencies and a moderate recovery bug that can retain unavailable ZFS instead of selecting supported Btrfs require changes.

5 open issues

_{Review effort: Lite}

Copilot

🔵 Needs a closer look

Active documentation issues remain: the driver name uses inconsistent btrfs capitalization, and the architecture diagram still describes the storage subsystem exclusively as ZFS despite documenting a Btrfs fallback.

2 open issues

3 resolved since last review

_{Review effort: Lite}

Copilot

🔵 Needs a closer look

The documentation still presents ZFS behavior as universal even though storage is now driver-dependent, leaving inaccurate explanations for the newly supported btrfs path.

2 open issues

_{Review effort: Lite}

Copilot

🟡 Changes recommended

The change still has an unresolved critical backend issue where an existing unsupported storage pool can be accepted and cause later operations to fail, along with documentation that contradicts fallback behavior.

3 open issues

_{Review effort: Lite}

ZFS is preferred, but WSL, SUSE, Arch, Fedora and others may ship without it. Instead of keying the fallback off WSL, pick the first driver LXD reports as supported (ZFS, then Btrfs), and fail the system check with a clear error if the pool is missing and neither is available. Record the pool's driver atomically, since it may be set after the daemon starts serving. Adopt an existing pool's driver if it is ZFS or Btrfs and reject anything else, recreate a missing pool on the preferred supported driver, and serialize ensureBackendReady so concurrent callers don't race to create the pool. Update the docs to describe the detection-based fallback. Closes: canonical#742 Signed-off-by: Lincoln Wallace <lincoln.wallace@canonical.com> wip Signed-off-by: Lincoln Wallace <lincoln.wallace@canonical.com>

Workshop no longer limits btrfs to WSL. It uses ZFS where LXD supports it and falls back to btrfs otherwise. Update the tutorial, reference, architecture and contributing pages to match. Signed-off-by: Lincoln Wallace <lincoln.wallace@canonical.com>

Copilot

🟡 Changes recommended

Approval is blocked by an unaddressed compatibility failure in the snapshot integration suite for Btrfs, along with incorrect handling of pending or errored storage pools and contradictory documentation.

6 open issues

1 resolved since last review

Reject existing pools whose drivers are no longer supported

_{Review effort: Lite}

 var (
-	storagePoolDriver = "zfs"
+	// storageDrivers lists the supported pool drivers, most preferred first.
+	storageDrivers = []string{"zfs", "btrfs"}


+		if pool.Status == api.StoragePoolStatusUnvailable {
+			return fmt.Errorf("%q storage pool unavailable: LXD cannot initialize it with the %q driver", storagePool, pool.Driver)
+		}


+It uses ZFS on systems where |ws_markup| detects that ZFS is available,
+and otherwise falls back to btrfs
+(for example on Windows Subsystem for Linux).


+For storage, it uses ZFS where |ws_markup| detects that ZFS is available,
+and otherwise falls back to btrfs.


jonathan-conder reviewed Aug 16, 2026

View reviewed changes

locnnil force-pushed the add-btrfs-fallback branch 2 times, most recently from 2fcccdc to 949bc5e Compare August 28, 2026 22:48

locnnil requested a review from jonathan-conder August 29, 2026 01:13

jonathan-conder requested changes Sep 4, 2026

View reviewed changes

jonathan-conder requested changes Sep 27, 2026

View reviewed changes

locnnil force-pushed the add-btrfs-fallback branch from e2fffd4 to 5958238 Compare October 5, 2026 14:48

locnnil requested a review from jonathan-conder October 5, 2026 14:56

jonathan-conder reviewed Oct 5, 2026

View reviewed changes

Comment thread docs/reference/workshops.rst Resolved

Comment thread internal/workshop/lxd/lxd_backend.go Resolved

locnnil requested review from jonathan-conder and a lite review from Copilot October 6, 2026 13:42

Copilot started reviewing on behalf of locnnil October 6, 2026 13:43

View session

Copilot AI reviewed Oct 6, 2026

View reviewed changes

Comment thread internal/workshop/lxd/lxd_backend.go Resolved

Comment thread docs/explanation/architecture/components.rst Resolved

Comment thread docs/reference/workshops.rst Resolved

Copilot AI lite review requested due to automatic review settings October 29, 2026 20:03

Copilot started reviewing on behalf of locnnil October 29, 2026 20:04

View session

Copilot AI reviewed Oct 29, 2026

View reviewed changes

Comment thread internal/workshop/lxd/lxd_backend.go Resolved

Copilot AI lite review requested due to automatic review settings November 3, 2026 14:42

Copilot started reviewing on behalf of locnnil November 3, 2026 14:42

View session

Copilot AI reviewed Nov 3, 2026

View reviewed changes

jonathan-conder mentioned this pull request Nov 4, 2026

LXD doesn't support ZFS 2.1, which in turn doesn't support ID-mapped mounts #1131

Open

Copilot AI lite review requested due to automatic review settings November 4, 2026 12:47

Copilot started reviewing on behalf of locnnil November 4, 2026 12:47

View session

Copilot AI reviewed Nov 4, 2026

View reviewed changes

Comment thread internal/workshop/lxd/lxd_backend.go Resolved

Comment thread internal/workshop/lxd/lxd_backend.go Resolved

Comment thread internal/workshop/lxd/lxd_backend_snapshots.go Resolved

Comment thread docs/explanation/architecture/components.rst Resolved

Copilot AI lite review requested due to automatic review settings November 5, 2026 15:52

Copilot started reviewing on behalf of locnnil November 5, 2026 15:52

View session

Copilot AI reviewed Nov 5, 2026

View reviewed changes

locnnil requested a lite review from Copilot November 5, 2026 17:03

Copilot started reviewing on behalf of locnnil November 5, 2026 17:04

View session

Copilot AI reviewed Nov 5, 2026

View reviewed changes

Comment thread internal/workshop/lxd/lxd_backend.go Resolved

Copilot AI lite review requested due to automatic review settings November 5, 2026 17:12

Copilot started reviewing on behalf of locnnil November 5, 2026 17:13

View session

locnnil requested a review from jonathan-conder November 5, 2026 17:16

Copilot AI reviewed Nov 5, 2026

View reviewed changes

locnnil requested a lite review from Copilot November 9, 2026 16:12

Copilot started reviewing on behalf of locnnil November 9, 2026 16:13

View session

Copilot AI reviewed Nov 9, 2026

View reviewed changes

Comment thread docs/contributing/development.rst Resolved

Comment thread docs/tutorial/part-1-get-started.rst Resolved

Copilot AI lite review requested due to automatic review settings November 11, 2026 19:43

Copilot started reviewing on behalf of locnnil November 11, 2026 19:43

View session

Copilot AI reviewed Nov 11, 2026

View reviewed changes

Copilot AI lite review requested due to automatic review settings November 11, 2026 19:48

Copilot started reviewing on behalf of locnnil November 11, 2026 19:48

View session

locnnil force-pushed the add-btrfs-fallback branch from 333d05a to dcdcf47 Compare November 11, 2026 19:49

Copilot AI reviewed Nov 11, 2026

View reviewed changes

Copilot AI lite review requested due to automatic review settings November 11, 2026 19:54

Copilot started reviewing on behalf of locnnil November 11, 2026 19:55

View session

Copilot AI reviewed Nov 11, 2026

View reviewed changes

Comment thread internal/workshop/lxd/lxd_backend.go Resolved

locnnil force-pushed the add-btrfs-fallback branch from dcdcf47 to fb51331 Compare November 11, 2026 20:02

locnnil added 2 commits November 12, 2026 12:35

locnnil force-pushed the add-btrfs-fallback branch from fb51331 to f190956 Compare November 12, 2026 15:35

Copilot AI lite review requested due to automatic review settings November 12, 2026 15:35

Copilot started reviewing on behalf of locnnil November 12, 2026 15:35

View session

Copilot AI reviewed Nov 12, 2026

View reviewed changes

		with automatic btrfs fallback otherwise,
		for example on Windows Subsystem for Linux (WSL).

		For storage, it uses ZFS where \|ws_markup\| detects that ZFS is available,
		and otherwise falls back to btrfs.

Conversation

locnnil commented Jul 21, 2026

Description

Self-review quick check

Docs

Uh oh!

jonathan-conder commented Jul 21, 2026

Uh oh!

jonathan-conder commented Jul 21, 2026

Uh oh!

locnnil commented Jul 22, 2026

Uh oh!

jonathan-conder commented Jul 22, 2026

Uh oh!

locnnil commented Aug 14, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jonathan-conder Aug 16, 2026

Choose a reason for hiding this comment

Uh oh!

locnnil Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

jonathan-conder commented Aug 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

locnnil commented Aug 29, 2026

Uh oh!

Uh oh!

Uh oh!

jonathan-conder Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jonathan-conder left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

jonathan-conder commented Aug 16, 2026 •

edited

Loading